Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add back support for reading from parent command, config file, or subcommand in flyte-cli #890

Merged
merged 2 commits into from
Mar 16, 2022
Merged

Add back support for reading from parent command, config file, or subcommand in flyte-cli #890

merged 2 commits into from
Mar 16, 2022

Conversation

wild-endeavor
Copy link
Contributor

@wild-endeavor wild-endeavor commented Mar 16, 2022 •
edited
Loading

TL;DR

If not set on the command line, flyte-cli was not reading the host/insecure flags from the config file. Behavior on master is:

(flytekit) ytong@argus:~/go/src/github.com/flyteorg/flytekit [flyte-sandbox] (master) $ cat ~/.flyte/local_sandbox
[platform]
url=localhost:30081
insecure=True

(flytekit) ytong@argus:~/go/src/github.com/flyteorg/flytekit [flyte-sandbox] (master) $ flyte-cli -c ~/.flyte/local_sandbox get-workflow -u wf:flytesnacks:development:core.control_flow.subworkflows.parent_wf:v0.3.56
DeprecationWarning: The command 'flyte-cli' is deprecated.
Config loading

################################################################################################################################
# flyte-cli is being deprecated in favor of flytectl. More details about flytectl in https://docs.flyte.org/projects/flytectl/ #
################################################################################################################################

Welcome to Flyte CLI! Version: 0.0.0+develop

Traceback (most recent call last):
  File "/Users/ytong/envs/flytekit/bin/flyte-cli", line 33, in <module>
    sys.exit(load_entry_point('flytekit', 'console_scripts', 'flyte-cli')())
  File "/Users/ytong/envs/flytekit/lib/python3.8/site-packages/click/core.py", line 1128, in __call__
    return self.main(*args, **kwargs)
  File "/Users/ytong/envs/flytekit/lib/python3.8/site-packages/click/core.py", line 1053, in main
    rv = self.invoke(ctx)
  File "/Users/ytong/envs/flytekit/lib/python3.8/site-packages/click/core.py", line 1659, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
  File "/Users/ytong/envs/flytekit/lib/python3.8/site-packages/click/core.py", line 1395, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/Users/ytong/envs/flytekit/lib/python3.8/site-packages/click/core.py", line 754, in invoke
    return __callback(*args, **kwargs)
  File "/Users/ytong/go/src/github.com/flyteorg/flytekit/flytekit/clis/flyte_cli/main.py", line 809, in get_workflow
    client = _get_client(host, insecure)
  File "/Users/ytong/go/src/github.com/flyteorg/flytekit/flytekit/clis/flyte_cli/main.py", line 277, in _get_client
    return _friendly_client.SynchronousFlyteClient(cfg, root_certificates=parent_ctx.obj["cacert"])
  File "/Users/ytong/go/src/github.com/flyteorg/flytekit/flytekit/clients/raw.py", line 116, in __init__
    self._channel = grpc.secure_channel(
  File "/Users/ytong/envs/flytekit/lib/python3.8/site-packages/grpc/__init__.py", line 2004, in secure_channel
    return _channel.Channel(target, () if options is None else options,
  File "/Users/ytong/envs/flytekit/lib/python3.8/site-packages/grpc/_channel.py", line 1479, in __init__
    _common.encode(target), _augment_options(core_options, compression),
  File "/Users/ytong/envs/flytekit/lib/python3.8/site-packages/grpc/_common.py", line 72, in encode
    return s.encode('utf8')
AttributeError: 'NoneType' object has no attribute 'encode'

Type

  • Bug Fix
  • Feature
  • Plugin

Are all requirements met?

  • Code completed
  • Smoke tested
  • Unit tests added
  • Code documentation added
  • Any pending items have an associated Issue

Complete description

  • Add back creation of a separate context (separate from the setup-config command).
  • If values for host/insecure are in the config object but not in the parent or sub command, then fill them in.
  • Missing insecure in with_parameters in PlatformConfig.
  • Don't pass root certs if creating an insecure client.

See the make_context code before the change here.

@wild-endeavor
bring back support for reading from all three locations
aeab883 
Signed-off-by: Yee Hing Tong <wild-endeavor@users.noreply.github.com>
@wild-endeavor wild-endeavor changed the title bring back support for reading from all three locations Add back support for reading from parent command, config file, or subcommand in flyte-cli Mar 16, 2022
@codecov
Copy link

codecov bot commented Mar 16, 2022 •
edited
Loading

Codecov Report

Merging #890 (1ea23be) into master (66e1007) will decrease coverage by 0.02%.
The diff coverage is 56.25%.

@@            Coverage Diff             @@
##           master     #890      +/-   ##
==========================================
- Coverage   86.51%   86.49%   -0.03%     
==========================================
  Files         230      230              
  Lines       22097    22107      +10     
  Branches     2465     2468       +3     
==========================================
+ Hits        19117    19121       +4     
- Misses       2576     2579       +3     
- Partials      404      407       +3
Impacted Files Coverage Δ
flytekit/configuration/__init__.py 95.01% <ø> (ø)
flytekit/clis/flyte_cli/main.py 44.29% <56.25%> (-0.05%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 66e1007...1ea23be. Read the comment docs.

kumare3
kumare3 previously approved these changes Mar 16, 2022
View reviewed changes
@wild-endeavor
nit
1ea23be 
Signed-off-by: Yee Hing Tong <wild-endeavor@users.noreply.github.com>
@wild-endeavor
Copy link
Contributor Author

sorry one more time? had a lint

@wild-endeavor wild-endeavor merged commit 46160bb into master Mar 16, 2022
myz540 pushed a commit to ProjectAussie/flytekit that referenced this pull request Apr 11, 2022
@wild-endeavor
Add back support for reading from parent command, config file, or sub…
c2c8866 
…command in flyte-cli (flyteorg#890)

Signed-off-by: Yee Hing Tong <wild-endeavor@users.noreply.github.com>
Signed-off-by: Mike Zhong <mzhong@embarkvet.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kumare3 kumare3 kumare3 approved these changes

@eapolinario eapolinario Awaiting requested review from eapolinario eapolinario is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@wild-endeavor @kumare3